Skip to content

Add clean if-file-deleted setting - #90

Merged
aomarks merged 9 commits into
mainfrom
clean-on-delete
Apr 8, 2022
Merged

Add clean if-file-deleted setting#90
aomarks merged 9 commits into
mainfrom
clean-on-delete

Conversation

@aomarks

@aomarks aomarks commented Apr 8, 2022

Copy link
Copy Markdown
Member

Adds a new option for the clean setting called if-file-deleted. In this mode, we clean output files if any of the input files have been deleted since the previous run.

This is useful for commands like tsc --build:

  • "clean": true (the default) is not a good option for TypeScript, because it either eliminates the benefits of incremental compilation, or causes your .tsbuildinfo to get out of sync, depending on whether you include your .tsbuildinfo file in the output array.

  • "clean": false is also not a good option for TypeScript, because it causes stale outputs to accumulate. This is because when you delete or rename a .ts source file, tsc itself does not automatically delete the corresponding .js file emitted by previous compiles.

  • "clean": "if-file-deleted" is a nice balance between fast and correct output. It lets you use tsc fast incremental compiles when a .ts source file is added or modified, but a clean build when a .ts source file is deleted.

Example

{
  "scripts": {
    "ts": "wireit",
  },
  "wireit": {
    "ts": {
      "command": "tsc --build --pretty",
      "clean": "if-file-deleted",
      "files": [
        "src/**/*.ts",
        "tsconfig.json"
      ],
      "output": [
        "lib/**",
        ".tsbuildinfo"
      ]
    }
  }

Fixes #70

Also

  • Renames CacheKey to ScriptState. I think this is a more clear name, because this data type is used for purposes other than caching (e.g. checking freshness, and now also for comparing input file names). It also maps more closely to the existing .wireit/<script>/state file, which is where we serialize state.

  • Some refactoring to ScriptState. We previously did not save any ScriptState at all when a script was uncacheable, and used a special UNCACHEABLE sentinel to represent that case. However, this meant that we had no record of the previous input files in the case where a script was uncacheable, which we now need. We now instead always write a state file, and set an uncacheable property on it.

  • Added a "Recipes" section to the README, for tool-specific guidance. Starting with TypeScript, since that was the motivation for this change.

@augustjk augustjk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found some residuals from CacheKey refactor and have a question about the behavior for considering input files of dependents noted in the test.

Comment thread src/executor.ts
Comment thread src/executor.ts Outdated
Comment thread src/test/clean.test.ts Outdated
@aomarks
aomarks requested a review from augustjk April 8, 2022 22:36

@AndrewJakubowicz AndrewJakubowicz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny comments! Nice!

Comment thread src/analyzer.ts
Comment on lines +239 to +241
wireitConfig.clean !== true &&
wireitConfig.clean !== false &&
wireitConfig.clean !== 'if-file-deleted'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if actionable, wondering if this should all be the same type? All strings instead of a bool and string combo.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered "always" and "never" instead of true and false. Somehow "never" feels stronger than false, and the potentially confusing thing is that we actually always do a clean build when restoring from cache. I'm not sure.

@augustjk augustjk Apr 8, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"on", "off"? or "enabled", "disabled"?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"on", "off"? or "enabled", "disabled"?

Those seem semantically equivalent to true and false, but I feel like it might be harder to remember those strings over true and false.

Comment thread README.md
Comment thread src/executor.ts Outdated
Comment thread src/executor.ts Outdated
@aomarks
aomarks merged commit d2dfccd into main Apr 8, 2022
@aomarks
aomarks deleted the clean-on-delete branch April 8, 2022 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Option to clean output only when input file deleted

3 participants